Skip to content

salt: T8973: remove package build due to feature removal - #1221

Merged
sever-sever merged 1 commit into
vyos:rollingfrom
c-po:salt-removal
Jun 10, 2026
Merged

salt: T8973: remove package build due to feature removal#1221
sever-sever merged 1 commit into
vyos:rollingfrom
c-po:salt-removal

Conversation

@c-po

@c-po c-po commented Jun 9, 2026

Copy link
Copy Markdown
Member

Change summary

As salt has been marked deprecated via T8056 and is thus deprecated in VyOS 1.5 and VyOS 1.4 it is time to remove it from the rolling release.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe): Package build removal due to feature removal

Related Task(s)

Related PR(s)

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

As salt has been marked deprecated via T8056 and is thus deprecated in VyOS 1.5
and VyOS 1.4 it is time to remove it from the rolling release.
@c-po
c-po requested review from asklymenko and sever-sever June 9, 2026 19:55
@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 8886f121-d5eb-4c2a-82de-36db2e43d9ca

📥 Commits

Reviewing files that changed from the base of the PR and between 0b27f39 and c598a12.

📒 Files selected for processing (7)
  • .github/workflows/trigger_rebuild_packages.yml
  • data/architectures/armhf.toml
  • data/live-build-config/hooks/live/18-enable-disable_services.chroot
  • scripts/package-build/salt/.gitignore
  • scripts/package-build/salt/build.py
  • scripts/package-build/salt/build.sh
  • scripts/package-build/salt/package.toml
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • ansible/ansible (manual)
💤 Files with no reviewable changes (7)
  • data/architectures/armhf.toml
  • scripts/package-build/salt/package.toml
  • scripts/package-build/salt/build.py
  • data/live-build-config/hooks/live/18-enable-disable_services.chroot
  • .github/workflows/trigger_rebuild_packages.yml
  • scripts/package-build/salt/.gitignore
  • scripts/package-build/salt/build.sh
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: build_iso
  • GitHub Check: codeql-analysis-call / Analyze (python)
  • GitHub Check: Mergify Merge Queue
  • GitHub Check: Mergify Merge Protections
  • GitHub Check: Summary
🧰 Additional context used
🔍 Remote MCP

Additional Context for Salt Package Removal PR Review

Based on my research, here is the relevant context for reviewing this PR:

Deprecation Timeline and Rationale

Salt minion was marked deprecated in task T8056, which aligns with the PR's reference to this task. Integration with Salt (service salt-minion) is deprecated and set to be removed in future VyOS versions — interest in that feature from the community and customers has been consistently low.

While there was no set schedule for removal initially, it was expected that VyOS 1.5 would be the last release to support it. This PR appears to implement that removal for the rolling release branch.

Current Status in VyOS 1.5

The feature was still included in VyOS 1.5.0 GA release, where the integration with Salt (set service salt-minion) is marked as deprecated and will be removed in future VyOS versions due to very low customer demand. This indicates that the PR aligns with the planned roadmap to completely remove Salt support in post-1.5 rolling releases.

Scope of Changes

The PR removes:

  • Workflow automation: Salt package rebuild triggers from GitHub Actions workflow
  • Architecture configuration: Additional repositories configuration for armhf architecture
  • System service configuration: The salt-minion.service disabling step from live build hooks
  • Build infrastructure: The entire salt package build script (76 lines removed) and package configuration, affecting the build system's ability to create salt Debian packages

Related Changes

The PR references a related pull request (vyos/vyos-1x#5262) in the vyos-1x repository, which likely contains corresponding configuration code removals in the main VyOS system itself.


🔀 Multi-repo context ansible/ansible

Linked repositories findings

ansible/ansible

  • lib/ansible/config/ansible_builtin_runtime.yml — plugin routing contains a saltstack entry (plugin_routing.connection.saltstack). Context lines show mapping/redirect entries. [::ansible/ansible::lib/ansible/config/ansible_builtin_runtime.yml:1-40]

  • lib/ansible/utils/encrypt.py — multiple uses of the identifier "salt" in password/vault/encryption logic (salt handling, PASSLIB/HAS_CRYPT detection). Relevant for crypt/salt string handling, not Salt the project. [::ansible/ansible::lib/ansible/utils/encrypt.py:1-200]

  • lib/ansible/_internal/_encryption/_crypt.py — crypt library/salt generation error handling (crypt_gensalt failure). [::ansible/ansible::lib/ansible/_internal/_encryption/_crypt.py:1-200]

  • lib/ansible/executor/playbook_executor.py — references variable named "salt" when getting vars (salt = var.get("salt", None)). [::ansible/ansible::lib/ansible/executor/playbook_executor.py:130-140]

  • lib/ansible/parsing/vault/init.py — reads VAULT_ENCRYPT_SALT config (custom_salt). [::ansible/ansible::lib/ansible/parsing/vault/init.py:1100-1180]

  • Tests/integration and unit tests:

    • test/integration/targets/cli/setup.yml and other integration tests use password_hash with explicit salt strings or test salt_size behavior. [::ansible/ansible::test/integration/targets/cli/setup.yml:1-40]
    • test/integration/targets/filter_core/tasks/password_hash.yml — tests behavior around salt_size and password_hash. [::ansible/ansible::test/integration/targets/filter_core/tasks/password_hash.yml:1-80]
    • test/integration/targets/lookup_password/tasks/main.yml — asserts that generated password files do not include " salt=" (ensures salt not present in lookup output). [::ansible/ansible::test/integration/targets/lookup_password/tasks/main.yml:1-120]
    • test/units/_internal/_encryption/test_crypt.py — unit tests for crypt and salt generation failure cases. [::ansible/ansible::test/units/_internal/_encryption/test_crypt.py:1-200]

Notes: Matches are mostly about "salt" as a cryptographic/value concept or the plugin name/key "saltstack", not the Salt stack package build removed in the vyos PR. No code referencing Salt package build artifacts or Salt services (e.g., salt-minion.service) was found in this repository search output.


📝 Walkthrough

Summary by CodeRabbit

  • Chores
    • Removed salt package build infrastructure, including CI/CD workflow triggers, build scripts, and configuration files
    • Removed salt-minion service disable command from initialization sequence
    • Removed additional repository configuration from armhf architecture

Walkthrough

The PR removes all Salt package build infrastructure from VyOS. The build definition and scripts in scripts/package-build/salt/ are deleted; the CI workflow no longer triggers on changes to salt sources; and system configurations that assume Salt presence (service disable rules, additional repositories) are removed.

Changes

Salt Build System Removal

Layer / File(s) Summary
Package build configuration and scripts removal
scripts/package-build/salt/package.toml, scripts/package-build/salt/.gitignore
The salt package TOML configuration (SCM metadata, build command, dependencies) and the .gitignore directory-specific ignore rules are deleted.
Build pipeline trigger removal
.github/workflows/trigger_rebuild_packages.yml
Path-based filter for scripts/package-build/salt/** is removed from the workflow's dorny/paths-filter list (lines 84–86). Conditional dispatch block that triggered build-package.yml for salt is removed (lines 235–240).
System and architecture configuration cleanup
data/architectures/armhf.toml, data/live-build-config/hooks/live/18-enable-disable_services.chroot
The additional_repositories configuration block in armhf architecture settings is removed. The systemctl disable salt-minion.service command is removed from the service-disabling sequence.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: removing the salt package build due to feature deprecation, matching the PR's primary objective.
Description check ✅ Passed The description is directly related to the changeset, explaining the deprecation rationale (T8056) and linking to the corresponding Phabricator task (T8973) and related PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
✨ Simplify code
  • Create PR with simplified code

Comment @coderabbitai help to get the list of available commands and usage tips.

@c-po c-po mentioned this pull request Jun 9, 2026
12 tasks
@mergify mergify Bot added the rolling label Jun 9, 2026
@mergify mergify Bot assigned c-po Jun 9, 2026
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

CI integration 👍 passed!

Details

CI logs

  • Config tests 👍 passed

@sever-sever sever-sever left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We agreed.to remove deprecated salt-minion feature.

@natali-rs1985 natali-rs1985 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean up salt-minion package build infrastructure after feature removal.

@sever-sever
sever-sever merged commit ec0afde into vyos:rolling Jun 10, 2026
13 of 14 checks passed
@vyos-bot vyos-bot Bot added mirror-initiated This PR initiated for mirror sync workflow mirror-completed and removed mirror-initiated This PR initiated for mirror sync workflow labels Jun 10, 2026
@c-po
c-po deleted the salt-removal branch July 2, 2026 05:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants